Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Support global variable #276

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Support global variable #276

wants to merge 2 commits into from

Conversation

rhdxmr
Copy link
Collaborator

@rhdxmr rhdxmr commented Feb 6, 2022

I am trying to support global variable in RedBPF.

Currently, users can make use of global variables in their probes since RedBPF
relocates .data or .bss section to bpf map at runtime. (Actually
relocating .bss isn't working correctly)

But I think the userspace API is not convenient enough yet.

This code is an exerpt from the example program of this draft PR.

#[repr(C)]
#[derive(Debug, Clone)]
struct Data {
    var: u64,
    var_wo_sync: u64,
}

let global = Array::<Data>::new(loaded.map(".bss").expect("map not found")).expect("can not initialize Array");
let gval = global.get(0).expect("global var value");
println!("w/ sync, w/o sync, pcpu = {}, {}, {}", gval.var, gval.var_wo_sync, pcpu_val.iter().sum::<u64>());

I think that loaded.map(".bss") or loaded.map(".data") using C structure is
not cool enough to access global variable. It feels like C code. I am used to
C and I like it but RedBPF is Rust library. 🙂

I'd like to see more abstract API that exposes individual global variables to
users.

Any suggestion and idea will be welcomed.

Thanks,

@rhdxmr rhdxmr marked this pull request as draft February 6, 2022 16:53
rsdy
rsdy reviewed Feb 7, 2022
View reviewed changes
examples/example-probes/src/global_var/mod.rs Outdated
@@ -0,0 +1,11 @@
use cty::*;

// This is where you should define the types shared by the kernel and user
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these variables should be declared on the crate level. Since it's the correct compilation unit for both userland and kernelspace, they should at least able to access the typing of what's declared here.

By doing this, we could come up with a proc macro, and/or generic Atomic<T> that wraps these variables in the userland with safe accessors, and leaves them alone in BPF.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's a good idea.
I didn't come up with this since I just considered collecting info of global variables in probes by ELF symbol table or BTF info.
Your suggestion shows me alternative designs. Cool.
Thanks,

@rhdxmr
Copy link
Collaborator Author

rhdxmr commented Feb 10, 2022

Hello @rsdy

Yesterday, I tried to make use of variables defined in mod.rs
Like you said I wanted to get whole information from the variable directly.

What I hoped to make was looked like this

let gvar = loaded.global(unsafe { &GLOBAL_VARIABLE }).expect("error on access gvar");

But I couldn't make it.

It is easy to get type info from the reference.
But I couldn't find out how to match the &GLOBAL_VARIABLE with the
variable defined in the resulting probe code.

If there is only one global variable in the probe code, we can assume what
&GLOBAL_VARIABLE means.
But what if there are two or three global variables?
How to know which reference is corresponding to which variables in the probe
code? I don't know.

Instead I found a workaround that I am not fully satisfied..

    let gvar = loaded
        .global::<u64>("GLOBAL_VAR")
        .expect("error on accessing gvar");

It looks pretty similar to the conventional Loaded::map method.

I still prefer the first API.. Do you have any idea to implement it?

@rhdxmr
Copy link
Collaborator Author

rhdxmr commented Feb 10, 2022

Compile errors in FC35 and archlinux are caused by change of kernel v5.16 header.
#282 solves that problem.

@rhdxmr rhdxmr marked this pull request as ready for review March 3, 2022 15:47
@rhdxmr
Copy link
Collaborator Author

rhdxmr commented Mar 3, 2022

@rsdy
I just converted this draft to PR.

It supports users to access global variables of BPF programs.

@rsdy
Copy link
Collaborator

rsdy commented Mar 3, 2022

Sorry for ignoring this. I actually read and re-read and re-read it and kept trying to come up with a way to make this API cleaner, but the only thing I can think of was using a macro to get the name with something like paste. Either way, it's kinda hackish, unfortunately.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rsdy rsdy rsdy approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rhdxmr @rsdy